Skip to content

feat(pcb): batch component placement atomically - #264

Merged
mixelpixx merged 2 commits into
mixelpixx:mainfrom
nordic-style:codex/feat/batch-component-placement
Aug 25, 2026
Merged

feat(pcb): batch component placement atomically#264
mixelpixx merged 2 commits into
mixelpixx:mainfrom
nordic-style:codex/feat/batch-component-placement

Conversation

@nordic-style

@nordic-style nordic-style commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Summary

Add set_component_placements, an additive PCB tool that sets the X/Y position and absolute rotation of multiple existing footprints atomically.

No linked issue.

Approach

  • Validate the complete batch, including duplicate and missing references, before any mutation.
  • For live KiCad, read one board snapshot, transform each footprint and all of its pads, graphics, and fields with the existing geometry primitives, and publish one UpdateItems request inside one KiCad commit.
  • For a closed board, transform every requested footprint in memory and perform one revision-checked atomic file write.
  • Keep single-item move and rotate behavior unchanged.

Compatibility and safety

This is an additive MCP tool and schema. The pcb_components count and all quoted total/toolset counts are updated.

Both paths are all-or-nothing. Live IPC verifies that the requested board is active and creates one undo step. The file fallback is used only when IPC is unreachable, rejects stale source content, validates every reference before writing, and preserves pads, graphics, fields, line endings, and unrelated board content.

Validation

  • cargo fmt --all -- --check
  • cargo test --workspace --locked --lib --tests — the full local core run reaches the same three update_symbols_from_library_* failures reproduced on origin/main; focused suites below pass
  • cargo test --workspace --locked --doc — not run
  • cargo clippy -p konnect-core --locked --all-targets -- -D warnings
  • cargo test -p konnect-core --locked --lib pcb_components::tests — 57 passed
  • cargo test -p konnect-ipc --locked --test footprint_transform_test — 3 passed on this branch
  • cargo test -p konnect --locked --test doc_tool_counts — 6 passed
  • Real KiCad 10 check: two existing footprints were submitted at their current placements; the response reported source: ipc, one undo step, unchanged live pad positions, and the board saved successfully

Review checklist

  • The diff is focused and contains no generated output, personal data, or unrelated cleanup.
  • New names follow docs/NAMING_CONVENTIONS.md; public renames include compatibility handling.
  • New behavior and failure paths have regression coverage.
  • File mutations are atomic and preserve unrelated content.
  • IPC mutations verify the requested board and do not leave partial batches.
  • Tool counts and docs are updated.

@nordic-style
nordic-style force-pushed the codex/feat/batch-component-placement branch from 4d3d487 to 5cb168f Compare August 17, 2026 23:02
@mixelpixx mixelpixx mentioned this pull request Aug 21, 2026
Merged
nordic-style and others added 2 commits August 25, 2026 15:59
@nordic-style @claude @mixelpixx
Merge main into codex/feat/batch-component-placement (rebase)
1aca19b 
Counts regenerate to 206 registered / 212 total, pcb_components 19.
types.rs keeps both sides' new structs (mixelpixx#265's IpcPad/IpcTitleBlock and
this branch's IpcFootprintPlacement). The footprint mock hand-merges the
two evolutions of spawn_footprints_mock: mixelpixx#265 made it stateful (GetItems
must observe what UpdateItems wrote, for the pad-readback test) and this
branch made it multi-footprint — it is now a Vec whose matching entries
are replaced per incoming update, plus this branch's BeginCommit and
EndCommit arms, and both sides' tests run against it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@mixelpixx @claude
fix(pcb): report the placements the board actually holds after a batch
43bb6b6 
Both paths of set_component_placements echoed the request back as
'placements'. That is two different answers for one final state: the
file path normalizes the root angle to KiCad's (-180, 180], so a
requested 270 was stored as -90 while the response said 270; the IPC
path never re-read anything after the commit.

The IPC path now derives its response from a post-commit read-back
(the mixelpixx#294/mixelpixx#232 standard), erroring if an updated footprint is missing
from it, and the file path re-parses each footprint's root (at ...)
from the content it just wrote. A test pins the decisive case:
requested 270, reported -90.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@mixelpixx
mixelpixx force-pushed the codex/feat/batch-component-placement branch from 5cb168f to 43bb6b6 Compare August 25, 2026 20:08
@mixelpixx

mixelpixx commented Aug 25, 2026

Copy link
Copy Markdown
Owner

Rebased onto main (~45 merges of drift), fixed up, live-verified — merging once CI is green. The design held up completely: nothing that landed since supersedes an atomic full-placement batch for existing footprints, and your all-or-nothing construction (validate → one snapshot → one UpdateItems inside one commit, bail-drops-the-commit) is exactly the house shape.

The rebase (43bb6b6 base commit): counts regenerate to 206 registered / 212 total, pcb_components 19 (of max 20 — you took the second-to-last slot). types.rs keeps both sides' additions (#265's IpcPad/IpcTitleBlock and your IpcFootprintPlacement). The one real hand-merge was spawn_footprints_mock: #265 had made the footprint mock stateful (a later GetItems observes what UpdateItems wrote — its pad-readback test depends on it) while you made it multi-footprint. It's now a Vec whose matching entries are replaced per incoming update, keeping your BeginCommit/EndCommit arms; both sides' tests pass against it.

One substantive fixup, on-theme for this repo: both success paths echoed the request back as placements. The IPC path now derives its response from a post-commit read-back, and the file path re-parses what it just wrote. The live run made the case better than any argument could: I requested rotation: 270 for R1 against a live KiCAD 10.0.5 session, and the response reported −90 — because that's what KiCad actually stored (it normalizes to (−180, 180]), and the read-back caught KiCad's own normalization. An echoed response would have said 270 while the saved file says −90: two answers for one state. A test pins the same case on the file path.

Live verification (ecc83 demo, results read from the file KiCad saved):

  • Batch move+rotate of R1 and C1 over IPC: source: "ipc", response values match the saved file exactly (positions and normalized rotations).
  • A batch naming a nonexistent reference (ZZ99) alongside a real one: refused atomically — "KiCad batch failed; changes dropped" — and the real footprint in that batch was untouched in the file.
  • One incidental find, not yours: a freshly-launched KiCAD answers AS_NOT_READY for a while, and the tool correctly refused without falling back to the file — the fix(pcb): harden live footprint pad readback #265-era fail-closed gating doing its job.

Full gate green at 1142 tests. Thanks for the careful atomicity work — the byte-identical-after-refusal tests are exactly the property that matters.

@mixelpixx
mixelpixx merged commit 7d53102 into mixelpixx:main Aug 25, 2026
9 checks passed
mixelpixx added a commit that referenced this pull request Aug 25, 2026
@mixelpixx @claude
Release v0.9.0
8648fe2 
The truth-in-responses release: what the tool reports is what the board
or sheet actually holds. Two new tools — update_footprints_from_library
(#232, TWT233) with its derived preserved-state reporting, and atomic
set_component_placements (#264) with post-commit read-back responses.
Junction dots are re-judged after every move, closing day-one #120
(#330, triglav-modular). Multi-unit components are handled whole by
every mutation (#273). Live pad readback errors instead of fabricating
and names all 96 representable layers (#265). Schematic layout measures
real symbol envelopes instead of origin hulls (#275). add_bus_entry
reports its sides from the sheet's geometry (#329). COMMERCIAL.md now
says plainly that internal use is free.

19 toolsets, 206 registered tools / 212 total.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@nordic-style @mixelpixx